Skip to content

Type trace axis ids#392

Draft
RedZapdos123 wants to merge 1 commit into
plotly:mainfrom
RedZapdos123:axis-id-enums
Draft

Type trace axis ids#392
RedZapdos123 wants to merge 1 commit into
plotly:mainfrom
RedZapdos123:axis-id-enums

Conversation

@RedZapdos123
Copy link
Copy Markdown

@RedZapdos123 RedZapdos123 commented Apr 28, 2026

Description:

Issue plotly/plotly.py#5583 points out that trace x_axis and y_axis setters are not self-explanatory because users have to discover values like "x2" and "y3" from examples.

This PR keeps the existing string-based axis ids, but gives them named types:

  • XAxisId
  • YAxisId

The trace fields now use those named axis-id string types, and the trace setters accept impl Into<XAxisId> / impl Into<YAxisId> so existing string inputs continue to work without introducing an enum or a hardcoded axis limit.

It also adds regression tests for both code paths:

  • derive-generated trace setters (Scatter)
  • manual trace setters (Contour)

This PR does not include the CI fixes from #397 which have been moved there unlike earlier.

Checklist:

  • I have reviewed all changes in this PR myself.
  • I have added relevant regression test cases for this fix.
  • I have run formatting checks in WSL using cargo +nightly fmt --all -- --check and cd examples && cargo +nightly fmt --all -- --check.
  • I have run linting checks in WSL using cargo clippy -p plotly -p plotly_derive -- -D warnings -A deprecated.
  • I have run focused tests in WSL using cargo test -p plotly axis_id --features static_export_chromedriver.
  • I have run package tests in WSL using cargo test -p plotly --features plotly_ndarray,plotly_image,static_export_chromedriver,debug.

@andrei-ng
Copy link
Copy Markdown
Collaborator

andrei-ng commented May 5, 2026

Hi @RedZapdos123,

Thank you for the PR. And thank you for the work on the CI fixes.

I propose that you make a new PR with the CI fixes to separate it from this PR.

Concerning the original issue, the axis IDs, can you give me a bit more context on why you opened an issue in plotly.py repo and referenced it here ? I didn't and don't understand the connection.

Regarding the fix itself. This crate is trying to follow plotly.py and plotly.js as much as possible. While using an Enum is indeed type safe, clear and idiomatic, it has the downside that it doesn't generalize well for the purpose of plots. There is already a long standing issue in this crate/repo that axis numbers are hardcoded for subplots up to only 8 axes (#184). This is an artifiial limit.

So I don't think using an Enum here brings much improvement but rather more complexity and I would prefer not to follow this approach.

@RedZapdos123 RedZapdos123 marked this pull request as draft May 10, 2026 15:19
Signed-off-by: Mridankan Mandal <xerontitan90@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants